refactor: improve type encapsulation in reassembly module#33
Merged
refactor: improve type encapsulation in reassembly module#33
Conversation
Prevents HashMap key mutation after insertion. Adds lower_ip(), lower_port(), upper_ip(), upper_port() accessors. Updates dispatcher and tests to use accessor syntax. Part of #12
… methods Consolidates segment mutation logic as methods on the type they operate on. Standalone functions in segment.rs become impl FlowDirection methods. All callers updated to method syntax. Part of #12
Deduplicates the flush-remove-notify pattern used by RST, FIN, expire, evict, and finalize into a single close_flow() method. Fixes minor inconsistency: expire/evict/finalize now count bytes_reassembled during final flush (RST/FIN already did). Part of #12
Segments with offsets beyond base_offset + max_receive_window are rejected with InsertResult::OutOfWindow. Default 1MB matches Suricata/Zeek/Snort industry defaults. Counter-only, no Finding generated (matches industry practice). Part of #12
Replaces triple lookup (.get + .get_mut + .remove) with single .remove() returning owned flow. Eliminates .expect() panic risk and simplifies memory accounting. Adds off-by-one boundary test for max_receive_window (window+1 rejected, window accepted). Part of #12
Adds TLS to protocol analysis, features, architecture diagram, and component table. Updates --tls flag description (no longer "coming soon"). Removes TLS from roadmap (implemented in #30). Adds max_receive_window to reassembly feature description.
There was a problem hiding this comment.
Pull request overview
This PR refactors the TCP reassembly module to strengthen type encapsulation and centralize mutation logic, while adding a configurable forward receive window to reject segments that are too far ahead of the current reassembly point.
Changes:
- Make
FlowKeyfields private and add read-only accessors to prevent mutation after use as aHashMapkey. - Move
insert_segment/flush_contiguousintoFlowDirectionmethods and addmax_receive_windowenforcement with a newInsertResult::OutOfWindow. - Deduplicate multiple “flush → remove → notify” paths into
TcpReassembler::close_flow()and alignbytes_reassembledcounting with delivered bytes.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/reassembly/flow.rs |
Privatizes FlowKey fields and adds read-only accessors. |
src/reassembly/segment.rs |
Moves segment insertion/flush into FlowDirection and adds out-of-window rejection. |
src/reassembly/mod.rs |
Wires new APIs, adds max_receive_window config/stats, and introduces close_flow() helper. |
src/dispatcher.rs |
Updates FlowKey field access to use new accessors. |
tests/reassembly_flow_tests.rs |
Updates tests to use FlowKey accessors. |
tests/reassembly_segment_tests.rs |
Updates tests for method-based segment operations; adds out-of-window unit test. |
tests/reassembly_engine_tests.rs |
Adds engine-level tests for out-of-window behavior and bytes_reassembled consistency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
FlowKeyfields private with read-only accessors — prevents HashMap key mutation after insertioninsert_segmentandflush_contiguousfrom standalone functions toFlowDirectionmethods — consolidates mutation logic on the typeclose_flowhelper onTcpReassembler— deduplicates 5 identical flush-remove-notify patterns (RST, FIN, expire, evict, finalize) into one method; fixesbytes_reassembledcounting inconsistencymax_receive_windowconfig (default 1MB, matching Suricata/Zeek/Snort) — rejects segments with offsets beyondbase_offset + max_receive_windowwith newInsertResult::OutOfWindowvariantCloses #12
Test plan
test_out_of_window_segment_rejected— unit test: rejects beyond window, accepts at boundary, off-by-one verifiedtest_out_of_window_segment_rejected_by_engine— engine test: small window (1000 bytes), verifies stats counter and edge acceptancetest_finalize_bytes_reassembled_consistent— regression guard:bytes_reassembledmatches total bytes delivered to handlercargo fmtclean,cargo clippycleanDeferred items